[WIP] FastBoot compatibility #819
Conversation
sivakumar-kailasam
changed the title
sivakumar-kailasam
changed the title
sivakumar-kailasam
changed the title
app/controllers/application.js
Outdated
| this.setProperties({ | ||
| 'flashError': this.get('nextFlashError'), | ||
| 'nextFlashError': null | ||
| }); |
There was a problem hiding this comment.
how is that related to fastboot?
There was a problem hiding this comment.
Not related. I just grouped couple of individual set calls. Should've probably kept in a different commit.
| url = `${protocol}://${host}`; | ||
| } | ||
| return url; | ||
| }), |
There was a problem hiding this comment.
comment here might be valuable to understand why we need this
app/routes/logout.js
Outdated
| activate() { | ||
| Ember.$.getJSON('/logout', () => { | ||
| fetch(`${this.get('appURL')}/logout`, () => { |
There was a problem hiding this comment.
should probably use ajax() for consistency for now
config/environment.js
Outdated
| }, | ||
|
|
||
| fastboot: { | ||
| hostWhitelist: ['crates.io', /^localhost:\d+$/] |
There was a problem hiding this comment.
can we add localhost only in dev mode?
app/mixins/fastboot-utils.js
Outdated
| * during Server Side Rendering(SSR) via FastBoot, | ||
| * the JS Fetch API doesn't work with absolute urls. | ||
| * This property gives the URL prefix for those network calls. | ||
| * */ |
There was a problem hiding this comment.
isn't it the other way around? it doesn't work with relative URLs, right?
There was a problem hiding this comment.
danngit, you're right. I don't think we need it anymore if I use ajax service.
app/controllers/me/index.js
Outdated
| @@ -1,20 +1,20 @@ | |||
| import Ember from 'ember'; | |||
| import ajax from 'ic-ajax'; | |||
|
|
|||
| const { inject: { service } } = Ember; | |||
There was a problem hiding this comment.
this will make it harder for the new modules codemod to transform it into an import from @ember/service... I'd rather just keep using the globals for now.
There was a problem hiding this comment.
Oh so that's why you've used full form everywhere. Got it!
There was a problem hiding this comment.
yep, and I don't multi-level destructuring that much 😉
|
Closing this PR in favor of smaller PRs to get this repo to be fastboot compatible and then land one PR with the fastboot specific changes. |
Pages: